Skip to content

fix: resolve npm's global root on versioned (kegged) node layouts - #130

Open
adrianco wants to merge 2 commits into
pacphi:mainfrom
adrianco:fix/global-root-versioned-node-layout
Open

fix: resolve npm's global root on versioned (kegged) node layouts#130
adrianco wants to merge 2 commits into
pacphi:mainfrom
adrianco:fix/global-root-versioned-node-layout

Conversation

@adrianco

Copy link
Copy Markdown

What

globalRoot() (src/lib/paths.mjs) falls back to an execPath-derived path whenever npm isn't spawnable. That fallback tried only <binDir>/../lib/node_modules and <binDir>/node_modules, which assumes the node executable sits directly under its prefix.

Homebrew doesn't work that way. process.execPath is symlink-resolved, so on a Homebrew install it is /opt/homebrew/Cellar/node/<version>/bin/node, not /opt/homebrew/bin/node. Both candidates miss, and globalRoot() throws cannot determine npm global root (is npm installed?) on a machine where npm is installed and working. mise, asdf, and nvm place versioned trees at a similar depth.

Why it matters

This isn't only a theoretical path. tests/kit/helpers/home-sandbox.mjs:40 points PATH at a directory that doesn't exist by design, so every sandboxed test takes the fallback. On a Homebrew-node macOS checkout, six tests in tests/kit/provider-cli.test.mjs fail — and they surface as opencode host-pick failures rather than as a path bug:

✖ pick --host claude,opencode enables + wires opencode ...
  ✗ Error: cannot determine npm global root (is npm installed?)
      at globalRoot (src/lib/paths.mjs:82:27)
      at rufloNodeModules (src/lib/paths.mjs:120:49)
      at nestedMcpServerPath (src/lib/opencode.mjs:78:19)

Reproducible in isolation:

env PATH=/nonexistent "$(which node)" -e "import('./src/lib/paths.mjs').then(m=>{try{m.rufloNodeModules()}catch(e){console.log(e.message)}})"

At runtime the same throw reaches ak host pick any time npm is off PATH.

Change

Walk the executable's ancestors (bounded to 5, and never probing the filesystem root, since /lib/node_modules is not a prefix) so a linked prefix is recovered, and honour npm's own documented npm_config_prefix override ahead of any derivation.

Nearest-first ordering is preserved: a keg-local tree still wins where one exists, and the sibling layout stays the last resort. npm root -g remains authoritative and is untouched.

The walk is split into two exported, injectable functions (globalRootCandidates, resolveGlobalRoot) so the layouts can be asserted as data — without installing node five different ways, and without touching the process-wide cache.

Verification

  • New tests/kit/paths-global-root.test.mjs — 8 tests covering Homebrew/linuxbrew kegged layouts, plain POSIX, nvm, the npm_config_prefix override, last-resort sibling ordering, a filesystem fixture tree, and the null-rather-than-guess path.
  • The six pre-existing provider-cli failures now pass: 17/17.
  • Full gate green on this machine: typecheck, lint, lint:md, build-check, and pnpm test (1461 tests, 0 failures).

No runtime dependency added; node: builtins only.

`globalRoot()` falls back to an execPath-derived path whenever `npm` is not
spawnable. That fallback only tried `<binDir>/../lib/node_modules` and
`<binDir>/node_modules`, which assumes the executable sits directly under its
prefix. Homebrew does not: `process.execPath` is symlink-resolved to
`<prefix>/Cellar/node/<version>/bin/node`, so both candidates miss and
`globalRoot()` throws "cannot determine npm global root (is npm installed?)"
on a machine where npm is installed and working. mise, asdf, and nvm place
their versioned trees at a similar depth.

This is not a theoretical path: the test sandbox points PATH at a directory
that does not exist by design (tests/kit/helpers/home-sandbox.mjs), so every
sandboxed run takes the fallback. Six tests in tests/kit/provider-cli.test.mjs
fail on any Homebrew-node macOS checkout, all surfacing as an opencode
host-pick failure rather than as a path bug. At runtime the same throw reaches
`ak host pick` whenever npm is off PATH.

Walk the executable's ancestors (bounded to 5, never probing the filesystem
root) so a linked prefix is recovered, and honour npm's own documented
`npm_config_prefix` override ahead of any derivation. Nearest-first ordering
is preserved, so a keg-local tree still wins where one exists, and the
sibling layout stays the last resort.

The walk is split into two exported, injectable functions so the layouts can
be asserted as data without installing node five different ways, and without
touching the process-wide cache.
…eparators

The Windows CI legs failed on the new suite. The production walk was correct
there — the candidate list contained `\opt\homebrew\lib\node_modules`, the
linked prefix this change exists to recover — but the assertions compared it
against hand-written '/opt/homebrew/lib/node_modules' literals, which only
match on a host where path.sep is '/'.

Rewritten to follow the convention in footprint-windows.test.mjs: run the REAL
win32 code path through an injected `path` implementation, so separator
handling is verified from any host rather than in whichever flavour the runner
happens to use. Expected values are now composed with the same implementation
under test, which is what makes a literal-vs-join mismatch impossible to
reintroduce. Three win32 cases are added — separator emission, the sibling
layout beside node.exe, and drive-root exclusion.

Also fixes a real discrepancy the Windows run exposed. The code claimed it
never probes the filesystem root, and the linuxbrew fixture appeared to
confirm it — but that path is deep enough that the ascent never reaches `/`,
so the assertion passed vacuously. A shallow executable such as /usr/bin/node
does reach it, and `/lib/node_modules` (or `C:\lib\node_modules`) was being
emitted as a candidate. The root is now genuinely skipped, matching the stated
contract, with both a POSIX and a win32 test that fail if it regresses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant